-
Notifications
You must be signed in to change notification settings - Fork 543
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Langmuir turbulence parameterization improvements #1034
Langmuir turbulence parameterization improvements #1034
Conversation
…rization In addition to the changes by @qingli411 as summarized below, I have added mod_def namelist parameters to control the Langmuir parameterization and eliminated the CESMCOUPLED and SDTAIL ifdefs by replacing them with if statements (with flags from the newly added namelist) wherever applicable. # Pull Request Summary This PR cleans up the calculation of Stokes drift and other diagnostics variables in WW3 for Langmuir turbulence parameterization in CESM. * Clean up the calculations of the surface Stokes drift (`USSX` and `USSY`) and surface layer averaged Stokes drift (`USSHX` and `USSHY`). The calculation of surface layer averaged Stokes drift and the tail contributions to both versions of Stokes drift are activated when `W3_CESMCOUPLED` is defined. * Change the input field of mixed layer depth `HML` to a more general surface layer depth `HSL`. This depth is used to calculate the surface layer averaged Stokes drift. When used in CESM, the surface layer depth is defined as 1/5 of the mixed layer depth. Now this 1/5 factor is applied to the mixed layer depth when importing the fields in the cap. * Clean up the diagnostic variables for Langmuir turbulence parameterization. Some variables are intermediate quantities and are not used in the parameterization. These variables are now deleted. The key variable diagnosed in WW3 is the surface layer averaged Stokes drift (`USSHX` and `USSHY`). The Langmuir enhancement factor (`LAMULT`) and the surface layer averaged Langmuir number (`LASL`) are now calculated when they are exported in the cap. `LASL` is needed for the Langmuir induced entrainment parameterization in CVMix but was not there in the cap code. So it is now added following the way `LAMULT` is defined. * This PR only changes the diagnostic variables in WW3 and shouldn't change the answers of WW3. Ideally, this PR shouldn't change the diagnostic variables to be used in CESM (`LAMULT`, `LASL`) as well. But given that the calculation of these variables are moved to the cap, and the way division by zero and physically invalid values (both should be rare) are handled, small changes to these variables are expected. Co-authored-by: qingli411 <[email protected]>
(Latest PR) Clean up diagnostic variables in WW3 for Langmuir turbulence parameterization
This reverts commit 7af1188.
NOTE: this PR is currently a draft for an initial review from @MatthewMasarik-NOAA and/or @JessicaMeixner-NOAA. I am also mentioning @mvertens and @DeniseWorthen, who may be interested in reviewing this early version. |
Awesome, @alperaltuntas, it's great to see this work! I have the UFS coupled tests running with it now. I'll post an update tomorrow. |
@alperaltuntas the runs I submitted yesterday had failures due to the mod_def's. This I should have known since the PR modifies |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alperaltuntas --- First and foremost - THANK YOU! These updates so far look good and I'm planning on taking an even closer look when I return from a conference at the end of July.
A couple of things that would be helpful in the meantime:
- Updates to model/inp/* and model/nml/* as appropriate to document new additions
- Add regression test for new features/output parameters
- Any updates that should go into the manual (Alternatively, if you'd create an issue with an assigned person + what needs to be document that will be sufficient, we'll just want to make sure we get it before the next public release).
Thank you @JessicaMeixner-NOAA for this initial look and comments. @alperaltuntas I'll start to add in the items Jessica requested, so there's nothing additional needed at this point. I'll work first to get the namelist and regression test implemented. After that maybe we can touch base and together try to tackle some supporting documentation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've suggest a load balancing exercise and some minor changes along with questions.
Frankly - I must not have submitted the suggestion - so please ignore this.
@mvertens Could you clarify? I am not sure what you mean by load balancing exercise, and which minor changes and questions you are referring to. |
@alperaltuntas I apologize for not responding earlier. I didn't see any issues w/ your changes in terms of the mesh cap. At some point, if this feature is used for the unstructured mesh, I think there will need to be a few changes to allow the new fields (usshx,usshy) to be written as gridded output. But I wouldn't view that as a requirement for this PR. @MatthewMasarik-NOAA I assume you've run (or will run) the UFS RTs with these changes prior to merge? |
@DeniseWorthen, yes, definitely. I'm going to make a few edits for the added regtest, which I plan to complete this week. Then @JessicaMeixner-NOAA and will review including running the UFS RTs. |
@MatthewMasarik-NOAA Thanks. I will be able to approve once all the UFS are shown to pass. |
… update to ww3_grid
Langmuir turbulence parameterization - PR polishing
@alperaltuntas Glad to see the polishing updates were merged in. Jessica will start her review on Monday. |
… fix2/sync-dev/unified * origin/mods/dev/ufs-weather-model: matrix_cmake_ncep: update modules for spack-stack prevent division by 0 in appendtail and add timers to meshcap (NOAA-EMC#1163)
…/unified Fix for ww3_tp2.22 plus updates from dev/ufs-weather-model
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
matrixCompFull.txt
matrixCompSummary.txt
matrixDiff.txt
Changes are in the mod_defs and out_grd as expected. Plan to do one last test of ensuring the new regtests reproduce itself, but this PR should be good to have a ufs-weather-model PR submitted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @alperaltuntas and @MatthewMasarik-NOAA for getting this in shape for eventual merge back to the develop branch.
@ukmo-ccbunney @mickaelaccensi @thesser1 - These updates will eventually be coming back to develop. If there's anything you see that needs to be immediately addressed, please feel free to chime in.
ww3_ounf.nml and ww3_shel.nml should also be updated to add this new output field |
@JessicaMeixner-NOAA and I discussed this previously (1034#discussion) with the outcome of deciding to limit the current PR to *.inp files, and had added issue #1111 for the expansion to *.nml. Does that seem reasonable? |
ok, it seems acceptable to not expand this current PR with nml if it's done in another one already scheduled |
|
Thanks for your confirmations @mickaelaccensi @ukmo-ccbunney |
@alperaltuntas @JessicaMeixner-NOAA |
w3iogomd.F90: fix IF-stmt indention
Ok, testing is complete on WM #2195. @JessicaMeixner-NOAA can you please merge this PR for us? |
d9b3172
into
NOAA-EMC:dev/ufs-weather-model
A belated thank you and congratulations to @qingli411 and @alperaltuntas for this very nice contribution to WW3, which was merged into the dev/ufs-weather-model branch last Friday! I'd also like to acknowledge @CarstenHansen, whose early posts to #669 provided helpful insight for implementing the namelist functionality. |
Pull Request Summary
This PR cleans up the calculation of Stokes drift and other diagnostics variables in WW3 for Langmuir turbulence parameterization in CESM, and removes the CESMCOUPLED ifdefs.
Description
Please also include the following information:
USSX
andUSSY
) and surface layer averaged Stokes drift (USSHX
andUSSHY
). The calculation of surface layer averaged Stokes drift and the tail contributions to both versions of Stokes drift are activated whenLMPENABLED
flag in mod_def file is set to True. Tail contribution is added in langmuir mixing computations ifSDTAIL
flag is set to True. HSLMODE switch determines how HSL (surface layer depth) is specified. HSLMODE=0 means testing (where HSL is set to 10m everywhere), HSLMODE=1 means HSL is received from the nuopc cap.HML
to a more general surface layer depthHSL
. This depth is used to calculate the surface layer averaged Stokes drift. When used in CESM, the surface layer depth is defined as 1/5 of the mixed layer depth. Now this 1/5 factor is applied to the mixed layer depth when importing the fields in the cap.USSHX
andUSSHY
). The Langmuir enhancement factor (LAMULT
) and the surface layer averaged Langmuir number (LASL
) are now calculated when they are exported in the cap.LASL
is needed for the Langmuir induced entrainment parameterization in CVMix but was not there in the cap code. So it is now added following the wayLAMULT
is defined.Issue(s) addressed
Commit Message
Clean up and refine Langmuir turbulence parameterization computations.
Authors: @qingli411, @alperaltuntas
Check list
Testing